Skip to content

Use CodeVersioning for EnC - #130159

Merged
rcj1 merged 24 commits into
dotnet:mainfrom
rcj1:enc-codeversioning
Jul 28, 2026
Merged

Use CodeVersioning for EnC#130159
rcj1 merged 24 commits into
dotnet:mainfrom
rcj1:enc-codeversioning

Conversation

@rcj1

@rcj1 rcj1 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Follow-up on #128338 (comment).

  • Implement existing runtime and cDAC functionality with EnC as a code version. Each subsequent edit corresponds to a single new ILCodeVersion with a single NativeCodeVersion.
  • EnC remains incompatible with ReJIT.
  • Added a Source field to ILCodeVersionNode to distinguish the sources.

Copilot AI review requested due to automatic review settings July 3, 2026 06:35
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the existing code-versioning infrastructure so Edit-and-Continue (EnC) edits are represented as explicit IL code versions (distinct from ReJIT), and updates DAC/cDAC and debugger plumbing to treat only true ReJIT versions as ReJIT.

Changes:

  • Adds a “source” discriminator for IL code versions (ReJIT vs EnC) plus an EnC version counter, and threads this through CoreCLR, data descriptors, and cDAC contracts.
  • Updates ReJIT enumeration / DAC-facing ReJIT APIs to exclude EnC code versions (and excludes the synthetic default version from ReJIT ID enumeration).
  • Adjusts IL retrieval paths in legacy cDAC/DAC DBI paths to read IL from the active EnC code version where appropriate; updates/extends unit tests and contract docs.
Show a summary per file
File Description
src/native/managed/cdac/tests/UnitTests/ReJITTests.cs Updates expected ReJIT ID enumeration behavior and mocks ICodeVersions.IsReJIT.
src/native/managed/cdac/tests/UnitTests/MockDescriptors/MockDescriptors.CodeVersions.cs Extends mock IL code version node layout with Source/EnCVersion; updates builder helpers.
src/native/managed/cdac/tests/UnitTests/CodeVersionsTests.cs Adds coverage asserting EnC/default versions are excluded from ReJIT detection and enumeration.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs Filters ReJIT-related legacy SOS DAC behaviors using ICodeVersions.IsReJIT.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs Fetches IL from active EnC code version when available; filters IL code version node APIs to ReJIT only.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs Uses active EnC IL for local signature parsing when applicable.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ILCodeVersionNode.cs Adds Source and EnCVersion fields to the ILCodeVersionNode contract data type.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CodeVersions_1.cs Implements ICodeVersions.IsReJIT based on the new ILCodeVersionNode.Source.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ICodeVersions.cs Introduces public CodeVersionSource and new ICodeVersions.IsReJIT API.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/Extensions/IReJITExtensions.cs Filters ReJIT ID enumeration to true ReJIT versions via cv.IsReJIT.
src/coreclr/vm/rejit.cpp Tags configured ReJIT IL code versions with CodeVersionSource::kReJIT; filters ReJIT ID queries.
src/coreclr/vm/method.hpp Makes EnC IL methods versionable via CodeVersionManager; enforces EnC/ReJIT mutual exclusion.
src/coreclr/vm/method.cpp Returns active EnC IL from CodeVersionManager in MethodDesc::GetILHeader.
src/coreclr/vm/encee.cpp Creates/activates EnC IL code versions via CodeVersionManager; records source and EnC version.
src/coreclr/vm/datadescriptor/datadescriptor.inc Adds ILCodeVersionNode.Source and ILCodeVersionNode.EnCVersion to the CDAC descriptor.
src/coreclr/vm/codeversion.h Adds CodeVersionSource enum and stores Source/EnCVersion on ILCodeVersionNode.
src/coreclr/vm/codeversion.cpp Implements ILCodeVersion::IsReJIT / GetEnCVersion, plus node accessors/mutators.
src/coreclr/debug/ee/functioninfo.cpp Treats only ReJIT (not EnC) explicit versions as “rejitted” for boundary mapping behavior.
src/coreclr/debug/ee/debugger.cpp Updates debugger ReJIT-related checks to ignore EnC versions; tags created versions as ReJIT.
src/coreclr/debug/daccess/request.cpp Filters DAC ReJIT data paths to only ReJIT IL code versions.
src/coreclr/debug/daccess/dacdbiimpl.cpp Uses CodeVersionManager to resolve EnC versions and EnC IL in DBI helper paths.
docs/design/datacontracts/CodeVersions.md Documents the new CodeVersionSource/IsReJIT concepts (but needs an EnCVersion row added).

Copilot's findings

  • Files reviewed: 22/22 changed files
  • Comments generated: 4

Comment thread src/coreclr/vm/codeversion.h
Comment thread docs/design/datacontracts/CodeVersions.md Outdated
Copilot AI review requested due to automatic review settings July 3, 2026 20:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 23/23 changed files
  • Comments generated: 6

Comment thread src/coreclr/vm/codeversion.cpp
Comment thread src/coreclr/vm/codeversion.cpp
Comment thread src/coreclr/vm/codeversion.cpp
Comment thread docs/design/datacontracts/CodeVersions.md Outdated
Comment thread docs/design/datacontracts/CodeVersions.md Outdated
@rcj1
rcj1 marked this pull request as ready for review July 4, 2026 16:00
Copilot AI review requested due to automatic review settings July 4, 2026 16:00
@rcj1
rcj1 requested a review from jkotas July 4, 2026 16:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

Comments suppressed due to low confidence (1)

src/coreclr/debug/daccess/request.cpp:4693

  • GetPendingReJITID now rejects the active ILCodeVersion unless ilVersion.IsReJIT() is true. However newly-requested ReJIT versions start in kStateRequested with CodeVersionSource still kUnknown (it isn't set until later in ConfigureILCodeVersion), so pending ReJIT queries will incorrectly return E_INVALIDARG. Reorder the checks to treat kStateRequested as a ReJIT-pending version even before Source is populated, and only use IsReJIT() to reject non-requested non-ReJIT (e.g., EnC) versions.
    CodeVersionManager::LockHolder codeVersioningLockHolder;
    ILCodeVersion ilVersion = pCodeVersionManager->GetActiveILCodeVersion(pMD);
    if (ilVersion.IsNull() || !ilVersion.IsReJIT())
    {
        hr = E_INVALIDARG;
    }
    else if (ilVersion.GetRejitState() == RejitFlags::kStateRequested)
    {
  • Files reviewed: 24/24 changed files
  • Comments generated: 3

Comment thread src/coreclr/debug/daccess/dacdbiimpl.cpp
Copilot AI review requested due to automatic review settings July 4, 2026 17:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 25/25 changed files
  • Comments generated: 4

Comment thread docs/design/datacontracts/CodeVersions.md Outdated
Comment thread docs/design/datacontracts/CodeVersions.md Outdated
Comment thread src/coreclr/vm/method.cpp Outdated
@jkotas
jkotas requested a review from noahfalk July 5, 2026 14:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

Comments suppressed due to low confidence (1)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs:5241

  • Null-output validation should include parameter names for consistency with the rest of this file (e.g., AreOptimizationsDisabled uses ArgumentException(..., nameof(...))). This keeps the resulting E_INVALIDARG easier to diagnose when surfaced through the PreserveSig COM boundary.
  • Files reviewed: 46/46 changed files
  • Comments generated: 0 new

Comment thread src/coreclr/debug/daccess/task.cpp
Comment thread src/coreclr/vm/prestub.cpp
Co-authored-by: Noah Falk <noahfalk@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 24, 2026 20:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

Comments suppressed due to low confidence (2)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/CodeVersions_1.cs:420

  • GetSource casts the raw ILCodeVersionNode.Source value directly to CodeVersionSource. If the target reports an unexpected value, this will leak an undefined enum value to callers. Consider validating/mapping unknown values back to CodeVersionSource.Unknown.
    src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ICodeVersions.cs:13
  • This change introduces new public API surface (public CodeVersionSource and new ICodeVersions virtual methods). Per dotnet/runtime policy, new public APIs require a linked api-approved issue/proposal. If this API isn't approved yet, consider making it internal (or otherwise non-public) until the API review completes.
  • Files reviewed: 46/46 changed files
  • Comments generated: 3

Copilot AI review requested due to automatic review settings July 24, 2026 21:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

Comments suppressed due to low confidence (2)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs:5244

  • GetEnCILCodeAndSig initializes *pLocalSigToken to mdtSignature (0x11000000). This is not mdSignatureNil and can be misinterpreted as a real token when no local sig exists or when no matching EnC version is found. The native DAC implementation initializes this output to mdSignatureNil (0) and only sets a token when present.

Initialize *pLocalSigToken to 0 (mdSignatureNil), and keep setting it to the actual local sig token only when found.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ICodeVersions.cs:15

  • This change introduces new public API surface (CodeVersionSource enum, plus ICodeVersions.GetSource / GetEnCVersion). The PR description doesn't reference any linked issue with the api-approved label, which is required for new public APIs in dotnet/runtime.

Either link the approved API review issue in the PR description, or make these APIs internal until API review is complete.

  • Files reviewed: 47/47 changed files
  • Comments generated: 0 new

Copilot AI review requested due to automatic review settings July 24, 2026 21:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

Comments suppressed due to low confidence (2)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ICodeVersions.cs:15

  • This change adds new public surface area to the cDAC Abstractions transport package (the public CodeVersionSource enum plus ICodeVersions.GetSource/GetEnCVersion). Per dotnet/runtime policy, new public API must be linked to an issue with the api-approved label (or the API should be kept internal until it’s approved). PR #130159 doesn’t reference an api-approved issue, so the approval status can’t be verified.
    src/native/managed/cdac/tests/UnitTests/CodeVersionsTests.cs:660
  • The new GetEnCVersion contract member is used by the legacy DacDbi implementation, but the updated unit test only validates GetSource. Adding assertions for GetEnCVersion here would cover both the EnC explicit version and the synthetic default version and make this test fail if the new behavior regresses.
  • Files reviewed: 47/47 changed files
  • Comments generated: 2

Comment thread docs/design/datacontracts/CodeVersions.md Outdated
Comment thread docs/design/datacontracts/data-descriptor-meanings.json Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 27, 2026 17:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

Comments suppressed due to low confidence (1)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ICodeVersions.cs:14

  • This change introduces new public API surface (CodeVersionSource plus ICodeVersions.GetSource / GetEnCVersion). Per repo policy, new public APIs require an approved API proposal (typically an issue labeled api-approved) or should be kept internal until approved. Please either link the approved issue/proposal in the PR description or adjust the surface area to be non-public if it is not intended as a supported API.
  • Files reviewed: 47/47 changed files
  • Comments generated: 0 new

Copilot AI review requested due to automatic review settings July 27, 2026 23:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

Comments suppressed due to low confidence (3)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/ICodeVersions.cs:14

  • This introduces new public API surface (CodeVersionSource + new ICodeVersions members in this PR) but the PR description doesn't reference any api-approved issue. Per repo policy, new public APIs must either be linked to an approved proposal/issue or be kept internal until approval.
    src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs:5244
  • Null output pointers are validated with a generic ArgumentException message and no parameter name, unlike neighboring methods in this file which include nameof(...). Including the parameter name makes failures actionable and consistent.
    src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/ClrDataFrame.cs:519
  • This condition is effectively trying to special-case EnC IL, but it currently treats any non-ReJIT explicit version as eligible. Narrowing it to Source == EnC avoids accidentally changing behavior if new explicit version sources are introduced in the future (or if Source is unexpectedly left as Unknown).
  • Files reviewed: 48/48 changed files
  • Comments generated: 0 new

@rcj1
rcj1 merged commit e614b71 into dotnet:main Jul 28, 2026
138 of 140 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants